1657 permanent fix for parallel organisation lookup by gibahjoe · Pull Request #1212 · digital-land/submit

gibahjoe · 2026-06-19T13:57:11Z

Description

Fixes a race condition where FetchOne could render a 404 response midway through a parallel middleware chain, allowing later middleware to continue and attempt a second response. Missing-record fetchOne results now pass a MiddlewareError(404) to the normal error handler, and the LPA overview chain fetches organisation info before parallel work that depends on it.

What type of PR is this? (check all applicable)

Related Tickets & Documents

Closes #1657

QA Instructions, Screenshots, Recordings

Manual reproduction was confirmed by temporarily restoring the race shape with a delayed parallel sibling. Before the fix, this produced:

Cannot set headers after they are sent to the client

After the fix, the same flow no longer crashes and the 404 is handled through the normal middleware error path.

Before

N/A - no UI change. Before fix, non-existent organisation requests could crash with a double-response error.

After

N/A - no UI change. Non-existent organisation requests now return a normal 404 flow.

Added/updated tests?

We encourage you to keep the code coverage percentage at 80% and above.

Yes
No, and this is why: Please replace this line with details on why tests have not been included
I need help with writing tests

Added/updated unit coverage for FetchOne missing-record handling and a slow sibling parallel() middleware scenario.

QA sign off

Code has been checked and approved
Design has been checked and approved
Product and business logic has been checked and proved

[optional] Are there any post-deployment tasks we need to perform?

None.

[optional] Are there any dependencies on other PRs or Work?

None.

Summary by CodeRabbit

Bug Fixes
- Improved handling when a single-item fetch returns no results so the request now passes a proper 404 error into the application’s error handling flow (instead of rendering the 404 directly).
- Ensured the request’s handler name is set before applying the “not found” fallback, and updated related tests to cover correct short-circuit behavior in parallel middleware scenarios.

coderabbitai · 2026-06-19T13:57:24Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e7c67f62-228e-4b37-9d75-af990f1453a9

📥 Commits

Reviewing files that changed from the base of the PR and between 92d2d3d and a06508f.

📒 Files selected for processing (1)

test/unit/middleware/middleware.builders.test.js

🚧 Files skipped from review as they are similar to previous changes (1)

test/unit/middleware/middleware.builders.test.js

Walkthrough

fetchOne's not-found fallback is changed to call next(new MiddlewareError('Not found', 404)) instead of rendering a 404 template inline. req.handlerName is now set before the zero-results fallback policy runs. Tests are rewritten and extended accordingly, including a new parallel test.

Changes

fetchOne 404 error propagation change

Layer / File(s)	Summary
fetchOne: MiddlewareError propagation and req.handlerName `src/middleware/middleware.builders.js`	Imports `MiddlewareError`, replaces the inline `res.status(404).render(...)` call with `next(new MiddlewareError('Not found', 404))`, updates the `'not-found-error'` policy comment, and sets `req.handlerName` before the zero-results fallback executes.
Test updates: fetchOne and parallel 404 behaviour `test/unit/middleware/middleware.builders.test.js`	Imports `MiddlewareError`, adds a `mockResponse()` helper, rewrites the `fetchOne` no-records test to assert `next()` receives the `MiddlewareError` and `res.status` is never called, and adds an async `parallel` test verifying the 404 short-circuit waits for a slow sibling middleware before propagating the error.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 Hop hop, no record found today,
No more rendering a 404 in-place!
I pass the error on its way,
With next() it runs the race.
MiddlewareError, neat and bright,
The parallel waits — then all is right! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title references issue 1657 and mentions 'permanent fix for parallel organisation lookup', which directly relates to the PR's main objective of fixing a race condition in parallel middleware execution for organisation lookup.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch 1657-permanent-fix-for-parallel-organisation-lookup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2026-06-19T13:58:48Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	70.38%	2467 / 3505
🔵	Statements	69.55%	2597 / 3734
🔵	Functions	63.78%	495 / 776
🔵	Branches	63.23%	1192 / 1885

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/middleware/middleware.builders.js	49.63%	39.13%	55.55%	48.88%	54-55, 59, 61, 86-93, 140-270, 285-287, 328-338, 420-422, 444-450

Generated in workflow #1529 for commit 862f879 by the Vitest Coverage Report Action

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/unit/middleware/middleware.builders.test.js`:
- Line 66: The assertion at line 66 (and the similar one at line 176) uses
`toHaveBeenCalledWith(new MiddlewareError(...))` which relies on Error instance
equality that doesn't reliably compare custom properties like statusCode on
Error subclasses. Instead of comparing against a fresh MiddlewareError instance,
change the assertion to explicitly verify that the first argument passed to the
`next` function is a MiddlewareError instance and explicitly assert that its
`message` property equals 'Not found' and its `statusCode` property equals 404.
Apply the same fix to both assertions at line 66 and line 176.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: fd4a325f-422b-453a-8da6-8e9e52dfd3ad

📥 Commits

Reviewing files that changed from the base of the PR and between 9ca8958 and 92d2d3d.

📒 Files selected for processing (2)

src/middleware/middleware.builders.js
test/unit/middleware/middleware.builders.test.js

…properties

…-lookup

gibahjoe added 2 commits June 19, 2026 13:55

temp refactor to recreate #1657

782e835

Refactor middleware to improve error handling and response management

92d2d3d

gibahjoe linked an issue Jun 19, 2026 that may be closed by this pull request

Permanent Fix for Parallel Organisation Lookup digital-land/config#1657

Closed

coderabbitai Bot reviewed Jun 19, 2026

View reviewed changes

Comment thread test/unit/middleware/middleware.builders.test.js Outdated

Enhance error handling in middleware tests to verify MiddlewareError …

a06508f

…properties

gibahjoe temporarily deployed to development June 22, 2026 09:18 — with GitHub Actions Inactive

pooleycodes approved these changes Jun 22, 2026

View reviewed changes

Merge branch 'main' into 1657-permanent-fix-for-parallel-organisation…

862f879

…-lookup

gibahjoe had a problem deploying to development June 22, 2026 10:29 — with GitHub Actions Failure

gibahjoe temporarily deployed to development June 22, 2026 15:57 — with GitHub Actions Inactive

gibahjoe merged commit aef64ed into main Jun 22, 2026
7 of 8 checks passed

gibahjoe deleted the 1657-permanent-fix-for-parallel-organisation-lookup branch June 22, 2026 16:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

1657 permanent fix for parallel organisation lookup - #1212

1657 permanent fix for parallel organisation lookup#1212
gibahjoe merged 4 commits into
mainfrom
1657-permanent-fix-for-parallel-organisation-lookup

gibahjoe commented Jun 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

gibahjoe commented Jun 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What type of PR is this? (check all applicable)

Related Tickets & Documents

QA Instructions, Screenshots, Recordings

Before

After

Added/updated tests?

QA sign off

[optional] Are there any post-deployment tasks we need to perform?

[optional] Are there any dependencies on other PRs or Work?

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

github-actions Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gibahjoe commented Jun 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

github-actions Bot commented Jun 19, 2026 •

edited

Loading